Add time measuring metrics for file ingestion in PerfContext #13219
Conversation
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| // These metrics usually have this pattern: "_[wait|delay]_*_[time|nanos]". | ||
| kEnableWait = 3, | ||
| // Starts enabling metrics that measure the end to end time of an operation. | ||
| // These metrics' names have keywords "time" or "nanos". Check other time |
There was a problem hiding this comment.
nit: "time" or time unit e.g, nanos, micros.
There was a problem hiding this comment.
Thanks for the suggestion! I specifically used "nanos" because that's the unit used for for all the time measuring perf context:
rocksdb/monitoring/perf_step_timer.h
Lines 60 to 66 in 1d919ac
| opts.allow_blocking_flush = true; | ||
| ASSERT_OK(db_->IngestExternalFile({file1}, opts)); | ||
| ASSERT_EQ(perf_context.file_ingestion_nanos, 0); | ||
| ASSERT_EQ(perf_context.file_ingestion_blocking_live_writes_nanos, 0); |
There was a problem hiding this comment.
I assumed your test ensure blocking live writes will happen so this proves your point.
There was a problem hiding this comment.
I actually haven't test it, let me add that part.
There was a problem hiding this comment.
Added now, thanks!
jowlyzhang
force-pushed
the
ingestion_time
branch
from
9a6da9d to
560d254
Compare
|
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
As titled. And also added some documentation for an approach to name perf context metrics that can help identify the starting
PerfLevelthat enables collecting it.Test Plan
Unit test